fix: handle write errors gracefully instead of panicking on /dev/full#7
Conversation
Replace println! with writeln!(io::stdout(), ...) and handle BrokenPipe silently, other I/O errors via stderr + exit 1. Matches the uutils convention used in coreutils' true.rs. Fixes uutils#5 Signed-off-by: SAY-5 <saiasish.cnp@gmail.com>
| println!("---\n{ast}"); | ||
| // Avoid `println!`, which panics on broken pipe / ENOSPC (e.g. `>/dev/full`). | ||
| // Match the uutils convention: silently exit on BrokenPipe, report other I/O | ||
| // errors to stderr and exit with failure. |
There was a problem hiding this comment.
We would do same things at anywhere. So the comment is not needed.
We should ayto-detect usage of (e)print(ln)! at CI instead.
There was a problem hiding this comment.
Relevant:
- https://rust-lang.github.io/rust-clippy/master/index.html#print_stdout
- https://rust-lang.github.io/rust-clippy/master/index.html#print_stderr
We can set those lints once we get rid of the macros.
| // Match the uutils convention: silently exit on BrokenPipe, report other I/O | ||
| // errors to stderr and exit with failure. | ||
| if let Err(e) = writeln!(io::stdout(), "---\n{ast}") { | ||
| if e.kind() == io::ErrorKind::BrokenPipe { |
There was a problem hiding this comment.
Would you avoid collapsible_if and if let ... = ... && instead?
Signed-off-by: SAY-5 <saiasish.cnp@gmail.com>
|
Done — replaced the print macros with structured logging. |
|
Seems okay to me. Ty for the contribution! I'll do a follow-up commit later to integrate this with |
|
It seems @SAY-5 does not appear as contributor by unknown reason on top page. |
Weird. If I had to guess, it could be because their commit email does not match their GitHub's, as their profile does not pop up on their commits. Elsewhere, maybe it's related to yesterday's outage, so we could ping support. @SAY-5 Can you verify the email here 18d5de2 is added to your account? If not, do so if you want GitHub to credit your work. |
|
Following up here: the merged commit now resolves to |
|
Probably. It's now happening to oech3's latest commit as well, so it seems GitHub is not resolving authorship on fast-forwarded contributions properly... |
3 participants
Fixes #5.
println!panics if the underlying write fails (e.g.>/dev/fullreturns ENOSPC, or stdout is a closed pipe). Replaced it withwriteln!(io::stdout(), ...), returning silently onBrokenPipeand reporting other I/O errors to stderr with exit code 1 — matching the convention used incoreutils/src/uu/true/src/true.rs.Added a Linux-gated regression test that pipes stdout to
/dev/fulland asserts the process does not panic (exit code != 2).